👋 Hello mathieucarbou, we appreciate your contribution to this project!

Generated by 🚫 dangerJS against 7014b39

@mathieucarbou - I remember that the reason for that is because ESP32|ESP32-S2|ESP32-S3 (Xtensa SoC) had a problem with ets_printf(). This code works for UART and also USB CDC logging, when Serial is actually a HWSerial or USBCDC port.

We can review if this is still valid.

@mathieucarbou - What SoC do you use with your projects?

@mathieucarbou - What SoC do you use with your projects?

I am currently testing with an esp32dev board, but the project has to also work for some other boards such as s2, s3, wt32_eth01, etc. This is a feature that will only be activated in debug mode to grab the boot logs from the ESP since users cannot connect easily to a USB computer.

I did the modification locally in the c file and tested with the esp32dev board and it works but I don't know the possible impact of this on other boards.

I was also intrigued by this comment in the header:

To install putc1, which is defaulted installed as ets_write_char_uart

So this is my understanding that putc1 (so a call to ets_printf) should already use ets_write_char_uart behind ?

Test Results

 76 files   76 suites   12m 44s ⏱️
 38 tests  38 ✅ 0 💤 0 ❌
241 runs  241 ✅ 0 💤 0 ❌

Results for commit 7014b39.

♻️ This comment has been updated with latest results.

I'll retest log output within all SoC using just ets_printf() to verify if it works for all possible Serial ports (UART/CDC).

Let set this PR on hold until the results are out.

Thanks a lot!

So this is my understanding that putc1 (so a call to ets_printf) should already use ets_write_char_uart behind ?

ets_install_putc1() and ets_install_putc2() should just define a pointer to a function that shall output anything that uses ets_printf(). In other words, ets_printf() calls the output function defined as putc1() or putc2().

Therefore, if your software needs to redirect log output, it could just change the function pointed by ets_install_putc1() for UART and ets_install_putc2() for USB CDC and use ets_printf() whenever logging.

ets_write_char_uart() outputs a single byte using exclusivly the UART peripheral - internal ROM implementation used by the system for outputting messages, such as when pressing BOOT and pulsing RESET to enter in BOOT mode.

ets_install_putc1() and ets_install_putc2() should just define a pointer to a function that shall output anything that uses ets_printf(). In other words, ets_printf() calls the output function defined as putc1() or putc2().

That's exactly my understanding also.

Therefore, if your software needs to redirect log output, it could just change the function pointed by ets_install_putc1() for UART and ets_install_putc2() for USB CDC and use ets_printf() whenever logging.

That's what does not work.

If you mean:

if your software needs to redirect log output when using ets_printf()

then I agree with you.

But this is not the use case explained above.

What I want and need is to redirect all calls to log_e, log_w, log_i, etc Basically, all Arduino logs, to a custom function. And for that, Arduino would need to call every time ets_printf(), which is not the case now because it directly calls instead the uart callback.

Memory usage test (comparing PR against master branch)

The table below shows the summary of memory usage change (decrease - increase) in bytes and percentage for each target.

@mathieucarbou - We will change esp32-hal-uart.c to only use ets_printf().

@SuGlider : thanks a lot !

Hello,

Just wanted to to drop a note that I was able to verify the feature in my application logger implementation (https://github.com/mathieucarbou/MycilaLogger)

#include <Arduino.h>
#include <MycilaLogger.h>
#include <StreamString.h>

Mycila::Logger logger;
StreamString logBuffer;

void setup() {
  Serial.begin(115200);
  while (!Serial)
    continue;

  Mycila::Logger::redirectArduinoLogs(logger);

  logger.setLevel(ARDUHAL_LOG_LEVEL_DEBUG);

  logger.forwardTo(&logBuffer);

  log_d("Arduino debug message");
  log_i("Arduino info message");
  log_w("Arduino warning message");
  log_e("Arduino error message");

  Serial.println("Captured Arduino logs:");
  Serial.println(logBuffer);
  logBuffer.clear();

  logger.debug("APP", "A debug message");
  logger.info("APP", "An info message");
  logger.warn("APP", "A warning message");
  logger.error("APP", "An error message");

  Serial.println("Captured Mycila logger logs:");
  Serial.println(logBuffer);
  logBuffer.clear();
}

void loop() {
  delay(500);
}

outputs:

That's a really super cool feature for apps with a non-dev user base in order to save the startup logs (up to a max amount) on disk when debug mode is activated in one's app.

I am using it like that:

Mycila::Logger logger;

class LogStream : public Print {
  public:
    LogStream(const char* path, size_t limit) : _path(path), _limit(limit) {
      File file = LittleFS.open(_path, "r");
      _logSize = file ? file.size() : 0;
      file.close();
      _accepting = _logSize < _limit;
    }
    size_t write(const uint8_t* buffer, size_t size) override {
      if (!_accepting)
        return 0;
      if (_logSize + size > _limit) {
        _accepting = false;
        return 0;
      }
      File file = LittleFS.open(_path, "a");
      if (!file)
        return 0;
      size_t written = file.write(buffer, size);
      file.close();
      _logSize += written;
      return written;
    }
    size_t write(uint8_t c) override {
      assert(false);
      return 0;
    }

  private:
    const char* _path;
    const size_t _limit;
    size_t _logSize = 0;
    bool _accepting = false;
};

[...]

logger.redirectArduinoLogs();
logger.forwardTo(&Serial); // Serial output
logger.forwardTo(webSerial); // WebSerial console at http://1.2.3.4/console
logger.forwardTo(new LogStream(YASOLR_LOG_FILE, 32 * 1024)); // first 32k of startup logs go on disk

Then I allow the user to download the startup logs for troubleshooting.